Skip to content

feat(lib): static image support #554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Soham-47
Copy link

@Soham-47 Soham-47 commented Jun 29, 2025

Closes #553

Description

Feature/static image support

Check List

Check all the applicable boxes:

  • I understand that my contributions needs to pass the checks;
  • If I created new functions / methods, I documented them and add type hints;
  • If I modified already existing code, I updated the documentation accordingly;
  • The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

SohamSaha47 and others added 13 commits June 29, 2025 14:31
- Add SlideType enum to distinguish between video and image slides
- Add static_image field to BaseSlideConfig with validation
- Update BaseSlide._save_slides to handle static images
- Add process_static_image utility function
- Update Qt player to display static images using QLabel
- Update HTML converter to use data-background-image for images
- Update PowerPoint converter to use add_picture for images
- Add example demonstrating static image usage
…e subprocess.run with shell=False, and add proper error handling
Copy link
Owner

@jeertmans jeertmans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Soham-47, thanks for your PR and efforts!

This is quite some amount of work, and I didn't have time to review everything, but I already had some questions on your work.

Also, I see that tests are currently failing, but I don't know if the PR was already ready to be reviewed or not.

Thanks!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have examples, but I would rather have them put inside the documentation (so they do not flood the main folder, and can be rendered inside the docs).

@@ -209,12 +251,16 @@ def __wrapper__(*args: Any, **kwargs: Any) -> Any: # noqa: N807
def apply_dedent_notes(
self,
) -> "BaseSlideConfig":
if self.dedent_notes:
if self.dedent_notes and self.notes is not None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Comment on lines +194 to +199
@field_validator("static_image")
@classmethod
def validate_static_image(cls, v: Any) -> Any:
if v is not None and cls.src is not None:
raise ValueError("Cannot set both 'src' and 'static_image'")
return v
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment on lines +165 to +192
src: Optional[str] = Field(None, description="Source video file path")
static_image: Optional[Union[str, Any]] = Field(
None, description="Static image file path or PIL Image object"
)
loop: bool = Field(False, description="Whether to loop the video")
notes: Optional[str] = Field(None, description="Speaker notes for this slide")
preload: bool = Field(True, description="Whether to preload the video")
aspect_ratio: Optional[float] = Field(None, description="Aspect ratio override")
fit: str = Field("contain", description="How to fit the video in the container")
background_color: Optional[str] = Field(None, description="Background color")
controls: bool = Field(True, description="Whether to show video controls")
autoplay: bool = Field(False, description="Whether to autoplay the video")
muted: bool = Field(True, description="Whether the video should be muted")
volume: float = Field(1.0, description="Video volume (0.0 to 1.0)")
playback_rate: float = Field(1.0, description="Video playback rate")
start_time: Optional[float] = Field(None, description="Start time in seconds")
end_time: Optional[float] = Field(None, description="End time in seconds")
poster: Optional[str] = Field(None, description="Poster image for the video")
width: Optional[str] = Field(None, description="Video width")
height: Optional[str] = Field(None, description="Video height")
class_name: Optional[str] = Field(None, description="CSS class name")
style: Optional[str] = Field(None, description="CSS style string")
data_attributes: Optional[dict[str, str]] = Field(
None, description="Additional data attributes"
)
custom_attributes: Optional[dict[str, str]] = Field(
None, description="Additional custom attributes"
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of those are HTML-specific variables, which I am not a great fan of to over-complexity the config file format. Maybe it would be better to create subclasses for format-specific configs. E.g., BaseSlideconfig would have three new fields: html_config: HTMLConfig, qt_config: QtConfig and pptx_config: PPTXConfig`. Those class will then be able to hold more fined-grained config options for each type of presentation format.

Comment on lines +260 to +261
# Forward reference for PIL Image type
PILImage = Any # Will be properly typed when PIL is imported
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use the TYPE_CHECKING + try import trick to have static type checker work with this.

b64 = b64encode(file.read_bytes()).decode("ascii")
mime_type = mimetypes.guess_type(file)[0] or "video/mp4"
mime_type = mimetypes.guess_type(file)[0] or "application/octet-stream"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation to change this?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you remove third-person conjugation in some places, but still add third-person in other places. Maybe we should unify this?

The historical reason why it's like that is that it is copied and pasted from RevealJS' docs, which is not really unified.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as for the example file: we should put those in the docs' assets.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@jeertmans jeertmans added enhancement New feature or request html-convert Related to converting to HTML slides pptx-convert Related to converting to PowerPoint slides lib Related to the library (a.k.a. module) qt Related to Qt (or its Python binding) labels Jul 2, 2025
@jeertmans jeertmans changed the title Feature/static image support feat(lib): static image support Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request html-convert Related to converting to HTML slides lib Related to the library (a.k.a. module) pptx-convert Related to converting to PowerPoint slides qt Related to Qt (or its Python binding)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Is it possible to create a slideshow with alternating images and videos using HTML?
3 participants